Repository navigation
Never patch a pnpm, yarn or vlt dependency the user patched - #2018
Draft
Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
Draft
Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
Conversation
A package the project patches itself is applied by the package manager on top of whatever tarball the lock names, so a Socket pin of it either fails the install or installs the user's diff stacked on the Socket patch. The CLI's consent gate (patch::user_patches::UserPatchIndex, #1646) already kept `scan` / `get` / `vendor` / `apply` from wiring such pnpm, yarn berry and patch-package packages unless --allow-patch-conflicts, but two consumers did not read that evidence: - vex discovery attested a pnpm, yarn, vlt (or npm / Bun) pin of such a package: a pin written before the patch was declared, or one an --allow-patch-conflicts run wrote. - the in-memory hosted engine (Node addon, hosted-bundle) has no gate in front of it and rewired them. One detector, three consumers. UserPatchIndex's pnpm / yarn / patch-package parse rules are factored into pure helpers (pnpm_yaml, yarn_manifest, yarn_lock, patch_package_dirs, patch_file), which the consent gate keeps using, plus UserPatchIndex::npm_family_in, which reads the same declarations through a ProjectView (disk, snapshot or memory): package.json's pnpm.patchedDependencies, yarn patch: descriptors and patch-package dirs, pnpm-workspace.yaml's patchedDependencies, the patches of the given pnpm locks (top-level map, patch_hash suffixes) and yarn.lock's user patch: locators. It leaves out npm's and Bun's own top-level patchedDependencies, which no other package manager reads and which #711 / #367 / #2010 already judge per resolution. - Discovery: a new vex::discover::user_patched pass marks every npm ref the index protects UnattestedKind::UserPatchedDependency (the ref stays, so rollback / remove / list still find it). The bytes the install runs are Socket's patch alone only when no user patch is declared, so a declared one always withholds the statement, with or without consent. - Hosted: engine::withhold_user_patched drops such candidates from every rewriter and records them skipped `user_patch_conflict` (the consent gate's code), plus a warning in memory. The in-memory engine always applies it; the CLI disk flow applies it unless --allow-patch-conflicts (it adds the Rush lock the consent gate does not read). - Vendored: unchanged. Every vendored entry point already runs the consent gate, which refuses (`vendor`, vendored writers) or skips (`scan`, `apply`) these packages. Override: the existing --allow-patch-conflicts, invocation-only. It already covered pnpm / yarn / patch-package at the consent gate and now also lifts the engine gate; vex never attests while the patch is declared. npm / Bun native patches stay refused even with consent, as before. vlt has no patch mechanism (vlt@1.6.2 has no patchedDependencies, patch_hash or `patch` command; vlt.json modifiers rewrite specs), and neither has yarn classic, so patch-package is what is read for both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Two follow-ups under the rule "a user patch applied on top means never attest". Bun (and npm) keys naming the pinned tarball itself. #2010 kept attesting a pin whose `patchedDependencies` key names the pin's own tarball (a `bun patch` made on top of the Socket pin, #1019), because discovery reused the hosted rewriter's per-dependency rule, where such a key is no conflict. Bun applies that patch over the Socket bytes, so the install never runs the Socket patch alone. Discovery now reads the keys against no target (any key for the name@version counts) and marks such a ref UserPatchedDependency with its own detail; npm discovery reads its keys the same way. bun_user_patched_dependency_pin_is_not_attested pinned the old attested behavior and now asserts the mark. The hosted rewriter is unchanged: it keeps such a pin (and confirms it on an in-sync re-run), while vex's plan omits it through the unattested mark. Workspace members. The consent gate walked workspace members and ancestor workspace settings; discovery and the in-memory engine read the project root only. UserPatchIndex's walk now runs over a ProjectView and is shared: discover() (the gate) is the walk over the disk with every ecosystem, npm_family_in() the same walk over any view with the npm-family declarations (pnpm, yarn, patch-package), so members' package.json pnpm.patchedDependencies, yarn patch: descriptors, patches/ folders and --patch-dir, and their pnpm/yarn files are read alike. The member glob walk is rewritten over the view (same depth, skip list, limits and link handling as the walkdir one), and an ancestor with any workspace marker is walked in both scopes. The walk never reads outside the git repository root (the nearest ancestor holding .git, inclusive; in memory the project itself): the ancestor walk stops there, as before, and a workspace member or literal --patch-dir beyond it now protects every npm package with an "outside the git repository" diagnostic instead of being read, in the gate too. Without a .git the gate's home-directory rule still bounds the ancestor walk. workspace_member_patches_agree_with_the_consent_gate checks discovery and the gate agree on member cases, on disk and in memory, and on a member beyond the repository root. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #2010 (
fix/npm-discovery-false-attestation).Owner decision: by default, a dependency the user patched themselves is not patched in any mode, for any package manager. Only an explicit user override patches it.
What was already there (and what wasn't)
The CLI's consent gate (
patch::user_patches::UserPatchIndex, #1646) already detects pnpmpatchedDependencies, yarn berrypatch:and patch-package for every npm-family project. Unless--allow-patch-conflictsis passed, it keepsscan(all modes),get,vendor,applyand the hosted/vendored writers from wiring those packages. Two consumers did not use that evidence:--allow-patch-conflictsrun wrote.hosted-bundle) has no consent gate in front of it, so it rewired those packages.One detector, three consumers
UserPatchIndex's pnpm, yarn and patch-package parse rules are split into pure helpers (pnpm_yaml,yarn_manifest,yarn_lock,patch_package_dirs,patch_file). The consent gate still uses them. A newUserPatchIndex::npm_family_in(view, locks)reads the same declarations through aProjectView, so it works on disk, on a snapshot or in memory.patchedDependencieskeys (name,name@version,name@range)package.jsonpnpm.patchedDependencies,pnpm-workspace.yamlpatchedDependencies, each pnpm lock's top-levelpatchedDependenciesmap and(patch_hash=…)key suffixes (root, Rush, and a ref's own member lock)patch:descriptors (Yarn'sbuiltin<compat/…>patches are excluded), every layer peeled, aliases resolvedpackage.jsondependency maps andresolutions;patch:locators thatyarn.lockresolves, including one that wraps the hosted tarball (#962)patches/<name>+<version>.patch, or a--patch-dirthat apackage.jsonscript namesThe new detector deliberately leaves out npm's and Bun's own top-level
patchedDependencies. No other package manager reads them, and #711, #367 and #2010 already judge them per resolution.vlt evidence:
vlt@1.6.2(latest, published 2026-11-03) contains nopatchedDependencies, nopatch_hashand nopatchcommand. Itsvlt.jsonmodifiersrewrite dependency specs, not package contents. A vlt project can therefore only patch through a postinstall tool such as patch-package, and that is what gets read.Behavior per mode
vex::discover::user_patchedpass marks every npm ref the index protects asUnattestedKind::UserPatchedDependency(reusing the variant from Stop attesting npm-family pins pnpm or a user patch overrides #2010; the note staysvex_npm_patched_dependency, with generic text). The ref stays, sorollback,removeandliststill find it.name@version, the package manager or its postinstall applies it on top of whatever tarball the lock names. The install therefore never runs the Socket patch alone, and the ref is never attested, with or without consent.engine::withhold_user_patchedremoves such candidates from every rewriter and records each as askippedrow with reasonuser_patch_conflict(the consent gate's code). In memory it also adds a warning.--allow-patch-conflictsis passed. There it mostly duplicates the consent gate, but it also reads the Rush lock, which the gate does not.vendorand the vendored writers refuse withuser_patch_conflict.scan --mode vendoredandapplyskip the package.tests/user_patches.rsalready covers this for pnpm, and the new agreement test below shows the vendored detector matches the discovery/hosted detector for every case.The override
The override is the existing invocation-only
--allow-patch-conflicts. It has no env or policy binding, and no new surface is added.CLI_CONTRACT.md (Existing project patches, Unattested references, the
vex_npm_patched_dependencyrow) and docs/usage.md are updated.Tests
Verified to fail before by temporarily making
npm_family_inreturn empty:vex::discover::user_patched::tests(3 of 4 fail before; the 4th is a negative control):package.jsonpnpmfield, lock map,patch_hashkey, range and bare keys; another version and npm's top-level key do not mark.package.jsonpatch:wrapping the hosted URL; ayarn.lockpatch locator; a builtin compat patch does not mark.--patch-dir.hosted_memory_engine::user_patched_npm_packages_are_never_pinned: pnpm (workspace andpackage.json), yarn berry (package.jsonandyarn.lock) and npm with patch-package. Each case is skippeduser_patch_conflictwith the lock untouched; another package's patch withholds nothing.in_process_redirect_pnpm::hosted_pins_a_pnpm_patched_dependency_only_with_explicit_consent:--allow-patch-conflicts: pinned;Added:
patch::user_patches::tests::npm_family_view_index_agrees_with_the_consent_gate. On disk and in memory, the view-based index and the consent gate agree on every case, and npm's native map belongs to the gate alone.Runs:
cargo test -p socket-patch-core --lib -- vex::discover:: patch::redirect:: vendor::pnpm vendor::yarn vendor::vlt vendor::npm vendor::bun hosted:: formats::pnpm formats::yarn patch::user_patches: 2512 passed. This includescommitted_fixture_corpus_matches_golden, with no golden changes.in_process_redirect_pnpm(42),in_process_redirect(152),hosted_memory_engine(50),hosted_memory_parity(42),user_patches(27),vex(48),e2e_vex_redirect(53), and libvex_sources/user_patches/scan::hosted(63). All pass.cargo clippy --locked --workspace --all-features -- -D warningsandcargo fmt --all -- --checkare clean.Follow-up commit: Bun pin-own-tarball keys, and workspace members
Two changes, both under the rule "a user patch applied on top means never attest".
1. A Bun key naming the pinned tarball is no longer attested. This is a
bun patchmade on top of the Socket pin (#1019). #2010 attested it, because discovery reused the hosted rewriter's rule, where such a key is no conflict. Bun applies that patch over the Socket bytes, so the install never runs the Socket patch alone.name@versioncounts. The ref is markedUserPatchedDependency, with its own detail. npm discovery reads its keys the same way.bun_user_patched_dependency_pin_is_not_attestedasserted the old attested behavior; it now asserts the mark.redirect_bun_patched_dependency_skipped. In both cases vex's plan omits the pin through the unattested mark, in-run VEX included.2. Workspace members are read the same way everywhere.
UserPatchIndex's walk now runs over aProjectViewand is shared by the CLI consent gate (discover(), on disk, every ecosystem) and by discovery and the hosted engine (npm_family_in(), on any view, npm-family declarations only).package.jsonpnpm.patchedDependencies, yarnpatch:descriptors,patches/and--patch-dir, and pnpm/yarn files..git; in memory, the project is its own root.--patch-dirbeyond it is not read. It protects every npm package with an "outside the git repository" diagnostic, in the gate too. This is a gate behavior change: previously the gate read such paths..git, the gate's existing rule still applies: the ancestor walk stops below the home directory.workspace_member_patches_agree_with_the_consent_gate: discovery and the gate agree on three member cases, on disk and in memory, and on a member beyond the repository root.Not done
🤖 Generated with Claude Code